Skip to content

fix(tui): preserve Ctrl+J newline in VTE terminals - #51546

Open
rvdavid-rbi wants to merge 2 commits into
NousResearch:mainfrom
rvdavid-rbi:fix/tui-vte-ctrlj-newline
Open

rvdavid-rbi wants to merge 2 commits into
NousResearch:mainfrom
rvdavid-rbi:fix/tui-vte-ctrlj-newline

Conversation

@rvdavid-rbi

Copy link
Copy Markdown

Summary

  • preserve Ctrl+J as a newline chord in VTE-based terminals by detecting VTE_VERSION
  • add regression coverage for Ubuntu/GNOME Terminal-style VTE environments
  • keep the existing fallback behaviour for bare POSIX terminals with no terminal fingerprint

Fixes #51545.

Verification

  • npm test -- --run src/__tests__/textInputPassThrough.test.ts
  • npm run typecheck
  • npm run build
  • live TUI smoke test in a VTE-like env confirmed Ctrl+J renders a two-line composer input while Enter remains submit

Copilot AI review requested due to automatic review settings June 23, 2026 21:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Preserves Ctrl+J as a newline chord in VTE-based terminals (e.g., GNOME Terminal/Console) by extending the TUI composer’s terminal fingerprint detection, with a regression test to prevent the Ubuntu/VTE behavior from reappearing.

Changes:

  • Detect VTE terminals via VTE_VERSION in shouldPreserveCtrlJNewline() to treat bare LF (\n) as an insert-newline chord rather than submit.
  • Add a Vitest regression case covering a VTE-like environment (TERM=xterm-256color, VTE_VERSION set).
  • Retain the existing “no fingerprint” fallback behavior (bare POSIX terminals keep Ctrl+J submitting).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ui-tui/src/components/textInput.tsx Adds VTE_VERSION fingerprint to preserve Ctrl+J newline handling.
ui-tui/src/tests/textInputPassThrough.test.ts Adds regression coverage for VTE environments and keeps the bare-POSIX fallback test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jun 23, 2026
@rvdavid-rbi

Copy link
Copy Markdown
Author

Thanks for the triage. To clarify scope: this PR fixes the TypeScript TUI renderer path in ui-tui/src/components/textInput.tsx, specifically shouldPreserveCtrlJNewline() and its Vitest coverage.

I saw the related PR #52102, which addresses the Python CLI predicate in cli.py. That appears to be the same root cause at a different layer rather than a direct duplicate.

Happy to do whichever maintainers prefer:

  1. keep this PR focused on the TypeScript renderer layer,
  2. expand this PR to also cover the Python CLI predicate, or
  3. close this in favour of a combined maintainer-preferred patch.

My preference is to keep both layers covered, since VTE detection seems relevant in both input paths.

@rvdavid-rbi
rvdavid-rbi marked this pull request as draft July 2, 2026 11:56
@rvdavid-rbi
rvdavid-rbi marked this pull request as ready for review July 2, 2026 11:56
rvdavid-rbi

This comment was marked as low quality.

@rvdavid-rbi
rvdavid-rbi marked this pull request as draft July 5, 2026 07:19
@rvdavid-rbi
rvdavid-rbi marked this pull request as ready for review July 5, 2026 07:19
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused TUI fix. Current main still lacks a VTE_VERSION branch in shouldPreserveCtrlJNewline() (ui-tui/src/components/textInput.tsx:128-150), while the composer sends bare-LF return events to the submit path unless that predicate is true (ui-tui/src/components/textInput.tsx:990-1000). The added predicate case and VTE regression test match that control flow.

The target files are unchanged from the PR's merge base through current HEAD (git diff --quiet 070ac2a71900f30b680ecafd4d68c162e9089a0a..HEAD -- ui-tui/src/components/textInput.tsx ui-tui/src/__tests__/textInputPassThrough.test.ts), so this remains a clean, focused salvage candidate. The related Python CLI path is separately addressed by #52102, consistent with the contributor's clarification.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
Eakkapoom-Name added a commit to Eakkapoom-Name/hermes-agent that referenced this pull request Sep 7, 2026
@Eakkapoom-Name

Copy link
Copy Markdown

Heads up: #98289 started as a superset of this PR. I wrote the fix before
finding this one, and it carried the same VTE_VERSION hunk in
shouldPreserveCtrlJNewline(). I have removed that hunk from #98289 so it now
depends on this PR rather than competing with it. #98289 keeps only the
extended-key advertisement in hermes-ink and the TERM fingerprints for
alacritty, foot, kitty and ghostty. This PR should merge first.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists and removed P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 7, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TUI: Ctrl+J does not insert newline in Ubuntu/VTE terminals

5 participants